Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#1320 Add module TypeScript definitions #2282

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

samueldmeyer
Copy link

@samueldmeyer samueldmeyer commented Dec 20, 2024

Fixes: #1320

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@samueldmeyer samueldmeyer changed the title #1320 Add module TypeScript definitions #1320 [WIP] Add module TypeScript definitions Dec 20, 2024
@samueldmeyer samueldmeyer changed the title #1320 [WIP] Add module TypeScript definitions #1320 Add module TypeScript definitions Dec 20, 2024
@srikant-ch5
Copy link
Contributor

Hi @samueldmeyer Thanks for the PR.

Could you please fix the DCO issue and also attach the issue 1320 like below in the PR above Dev Certificate ?

Screenshot 2025-01-08 at 11 21 55 AM

@samueldmeyer samueldmeyer force-pushed the add-type-defs branch 2 times, most recently from 382ada2 to 65fbffe Compare January 8, 2025 17:01
@samueldmeyer
Copy link
Author

@srikant-ch5 I've fixed the DCO issue, updated the message, and rebased on the commits added since I started this PR

@samueldmeyer
Copy link
Author

@srikant-ch5 Is there anything else I need to do for this pull request?

@tomlyn
Copy link
Member

tomlyn commented Jan 23, 2025

Hi @samueldmeyer -- I think I was supposed to look through this. Apologies for not doing so, I've been concentrating on some of the features you asked for.

At first glance I wonder whether we need some tests (presumably Jest tests) to test at least some of this?

Also, I think @matthoward366, @nmgokhale and @caritaou should review the Common Properties parts of this since they will need to maintain this Type script API going forward.

@samueldmeyer
Copy link
Author

@tomlyn I added tests using tsd, which is the standard for TypeScript definitions inside the library. I didn't test everything because a lot of that would simply be a repeat of what is in the definitions, but I did test the parts that are more complicated and generally made it extensible for adding tests. I added the tests to npm test, but you could tell me another way you want it run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add module TypeScript definitions and compatibilty
3 participants